-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(table): read selectable prop from each row by default #1321
base: develop
Are you sure you want to change the base?
Conversation
@kimjaej - Hi there! Thanks for adding a PR in Sage. Could you provide a bit more information in the Conversation portion of the PR so that we may better understand and review your work so you can merge? Specifically, how do we test your change in Storybook, are there any visuals you can provide us a before/after, and does this change impact |
@@ -278,7 +278,7 @@ export const Table = ({ | |||
cells={cells} | |||
schema={schema} | |||
selected={selfSelectedRows === SELECTION_TYPES.ALL || selfSelectedRows.includes(rowId)} | |||
selectable={selectable} | |||
selectable={!!(row.selectable || selectable)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious about the double negation here. Is it the case that one or both of row.selectable
and selectable
aren't set as booleans? I'm basing this all off of the propTypes defs enforcement; unsure of what we're actually finding in-app.
b5f45fa
to
d0f2ff2
Compare
a0cb960
to
041b58b
Compare
11dde9c
to
48f6933
Compare
81a9318
to
da9b7c3
Compare
7caaef2
to
3359401
Compare
Description
Read selectable prop from the row by default
Screenshots
Testing in
sage-lib
Testing in
kajabi-products
Related
https://kajabi.atlassian.net/browse/MAN-2626